Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move --p2p-host validation to CLI validation #8158

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Move --p2p-host validation to CLI validation #8158

merged 2 commits into from
Jan 23, 2025
+196 −122

Conversation

Gabriel-Trintinalia
Copy link
Contributor

PR description

Fail fast if --p2p-host is invalid.

Changes:

  • Added validation for --p2p-host in the CLI.
  • Replaced p2pHostMayBeLocalhost test with p2pHostMustBeAnIPAddress.
  • Moved p2pOptions validation to the appropriate class.
  • Relocated p2pOptions tests to a new class.

Before:

2025-01-23 12:24:07.088+11:00 | pool-8-thread-1 | INFO  | TrieLogPruner | Added 1 trie logs to prune queue. Commencing pruning of eligible trie logs...
2025-01-23 12:24:07.088+11:00 | pool-8-thread-1 | INFO  | TrieLogPruner | Pruned 0 trie logs
2025-01-23 12:24:07.088+11:00 | main | INFO  | TrieLogPruner | Trie log pruner queue preload complete.
2025-01-23 12:24:07.088+11:00 | main | INFO  | RunnerBuilder | Resolved 4 bootnodes.
2025-01-23 12:24:07.091+11:00 | main | INFO  | RunnerBuilder | Detecting NAT service.
2025-01-23 12:24:07.105+11:00 | main | ERROR | Besu | Failed to start Besu
java.lang.IllegalArgumentException: valid advertisement host required
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:143)
	at org.hyperledger.besu.ethereum.p2p.discovery.PeerDiscoveryAgent.validateConfiguration(PeerDiscoveryAgent.java:395)
	at org.hyperledger.besu.ethereum.p2p.discovery.PeerDiscoveryAgent.<init>(PeerDiscoveryAgent.java:120)
	at org.hyperledger.besu.ethereum.p2p.discovery.VertxPeerDiscoveryAgent.<init>(VertxPeerDiscoveryAgent.java:79)

Now:

2025-01-23 14:03:40.660+11:00 | main | INFO  | Besu | Starting Besu
2025-01-23 14:03:41.208+11:00 | main | ERROR | Besu | Failed to start Besu
picocli.CommandLine$ParameterException: The provided --p2p-host is invalid: INVALID_HOST
	at org.hyperledger.besu.cli.options.P2PDiscoveryOptions.validateP2PHost(P2PDiscoveryOptions.java:266)

@Gabriel-Trintinalia Gabriel-Trintinalia enabled auto-merge (squash) January 23, 2025 03:11
usmansaleem
usmansaleem approved these changes Jan 23, 2025
View reviewed changes
Copy link
Member

@usmansaleem usmansaleem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Gabriel-Trintinalia Gabriel-Trintinalia merged commit a1e087d into hyperledger:main Jan 23, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@usmansaleem usmansaleem usmansaleem approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Gabriel-Trintinalia @usmansaleem